Skip to content

Conversation

@Xnopyt
Copy link
Collaborator

@Xnopyt Xnopyt commented Aug 15, 2025

This updates DbScheduler to handle deadlocks better when claiming jobs. This should prevent potential issues when running multiple workers for the MonitorCommand.

This makes the following changes:

  • Restructures the query to SELECT ... FOR UPDATE and the update the rows as claimed, which should prevent unnecessary locks when indexes are properly configured for the table.
  • Optionally allows SKIP LOCKED to be used on the query. This change allows jobs to be considered claimed by a worker as soon as the row becomes locked.
  • Support for automatic retry with backoff if a deadlock does occur. This uses stechstudio/backoff library and requires a Backoff object to passed into the constructor.

@Xnopyt Xnopyt self-assigned this Aug 15, 2025
@Xnopyt Xnopyt added the minor-version Release SHOULD be minor version increment label Aug 15, 2025
@Xnopyt Xnopyt force-pushed the dbscheduler-deadlock branch 2 times, most recently from dc5ac44 to cc585e0 Compare August 15, 2025 11:10
@codecov
Copy link

codecov bot commented Aug 15, 2025

Codecov Report

❌ Patch coverage is 97.87234% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 68.07%. Comparing base (85f37a1) to head (51b5aaa).
⚠️ Report is 5 commits behind head on 2.x.x.

Files with missing lines Patch % Lines
src/Scheduler/DbScheduler.php 97.87% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##              2.x.x      #15      +/-   ##
============================================
+ Coverage     65.43%   68.07%   +2.63%     
- Complexity      120      132      +12     
============================================
  Files            10       10              
  Lines           434      473      +39     
============================================
+ Hits            284      322      +38     
- Misses          150      151       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@chrisminett
Copy link
Member

Does it need to use an extra dependency for retries? Seems like you've got all the logic here already, apart from the basic while() and counter. Retries could then just be an optional parameter ($maxAttempts = 1) with no need for passing in a dependency or having a different method.

Also, looks like the examples in README.md and /examples may need updating for the changes to DbScheduler.

@Xnopyt Xnopyt force-pushed the dbscheduler-deadlock branch 2 times, most recently from e90040c to 1ca7bce Compare August 15, 2025 16:16
@Xnopyt
Copy link
Collaborator Author

Xnopyt commented Aug 15, 2025

@chrisminett Examples and README have been updated with the new usage of DbScheduler

ORDER BY
scheduled_ts DESC
LIMIT {$batchSize}";
LIMIT {$batchSize}
Copy link
Collaborator

@amaanhassim amaanhassim Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we be binding batchSize just to be safe?

Limit :batchSize
and then add the batchSize through the stuff below where you are doing minimumPickup

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not quite that simple. The default type PDO uses for binding is string (which would be a syntax error here as it would be quoted) and phlib/db does not support setting the type of a bind.
This should be safe as-is, we are using strict types and $batchSize is type hinted as an int, so it shouldn't be possible to pass an unsafe value here. The :minimumPickup binding was added back in the PHP 5.4 days before strict typing was in the language.

@Xnopyt Xnopyt force-pushed the dbscheduler-deadlock branch from 1ca7bce to 74de03c Compare August 18, 2025 10:36
@Xnopyt Xnopyt force-pushed the dbscheduler-deadlock branch from 74de03c to 51b5aaa Compare August 19, 2025 09:52
@Xnopyt Xnopyt merged commit 5722bb2 into phlib:2.x.x Aug 19, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

minor-version Release SHOULD be minor version increment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants